fix: binary-safe HTTP body buffering for multi-segment POST/PUT#1573
Open
muralidhar-challa wants to merge 3 commits into
Open
fix: binary-safe HTTP body buffering for multi-segment POST/PUT#1573muralidhar-challa wants to merge 3 commits into
muralidhar-challa wants to merge 3 commits into
Conversation
When a POST or PUT request body spans multiple TCP segments (~1460 bytes each), only the first segment's body bytes were used — subsequent segments were silently dropped because this.read was cleared after the first \r\n\r\n delimiter match, and no more headers trigger re-processing. This caused "Invalid JSON" errors for any request body larger than approximately one MTU. Fix: - On the first segment: if Content-Length is present and the body is incomplete, store the partial body in this._xb with a deferred callback instead of firing fetch() immediately. - On subsequent segments: accumulate incoming data into this._xb.buf. Once Content-Length is satisfied, decode and invoke the deferred fetch. - Extract the fetch+response handling into _dispatch_fetch() to avoid duplicating it in both the normal and deferred code paths.
Addresses reviewer feedback on PR copy#1567: - Buffer incoming TCP data as raw bytes (Uint8Array) instead of text-decoding everything. Search for \r\n\r\n in binary, text-decode only the header portion. This prevents corruption of binary POST bodies and ensures the first \r\n\r\n in headers (not in body) is used as the separator. - Replace ad-hoc new TextEncoder()/new TextDecoder() with module-scoped singletons (textEncoder, textDecoder). - Refactor _dispatch_fetch into a standalone dispatch_fetch(conn, ...) helper that takes the connection explicitly, avoiding incorrect binding between the connection and adapter objects. - Add comment noting that Transfer-Encoding: chunked is unsupported. - Add unit tests (tests/devices/fetch_network_post.js): * Small POST body in one segment * Large POST body split across 3 segments * Binary body containing embedded CRLFCRLF bytes * GET request (no body) * Headers split across 2 segments
Add a POST handler to the test server that echoes the received body length, and a test case that POSTs 3000 bytes (> ~1460 byte MTU) from the buildroot guest via curl + dd. Verifies the full body reaches the server after the binary-safe buffering fix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a POST or PUT request body spans multiple TCP segments (~1460 bytes each), only the first segment's body bytes were used — subsequent segments were silently dropped because
this.readwas cleared after the first\r\n\r\ndelimiter match, and no more headers trigger re-processing.Fix:
Uint8Array) instead of text-decoding everything. Search for\r\n\r\nin binary, text-decode only the header portion. Body stays as binary.this._xbwith a deferred callback. Subsequent segments accumulate until Content-Length is satisfied, then dispatch.new TextEncoder()/new TextDecoder()with module-scoped singletons.dispatch_fetch(conn, ...)to avoidthisbinding issues.Tests:
tests/devices/fetch_network_post.js(small POST, large split POST, binary body with embedded CRLFCRLF, GET, split headers)tests/devices/fetch_network.js(POSTs 3000-byte body from buildroot guest, verifies full body arrives)Build: 0 errors, 0 warnings. ESLint: clean.